Renderer transformation refactor by PringlesGang · Pull Request #165 · CommitteeOfZero/impacto

PringlesGang · 2025-04-27T22:12:16Z

A refactor of the renderer to make more use of generic transformation matrices instead of a myriad purpose-built overloads.
Also adds useful functions for generating these transformation matrices, monadically transforming RectFs and CornerQuads, and reduces code duplication within and across the renderer implementations.
A number of internal renderer components have also been overhauled to become managed and make use of more modern C++.

Implements DrawMaskedSprite for both DirectX9 and Vulkan.

Should (obviously) not visually change anything for the worse.

PringlesGang · 2025-05-07T23:01:04Z

Best of luck Dext; you're gonna need it o7

KKhanhH · 2025-05-08T04:16:17Z

-  std::array<uint16_t, MaxMvlIndices> MvlIndices;
-  int MvlIndicesCount;
+  std::vector<VertexBufferSprites> MvlVertices;
+  std::vector<uint16_t> MvlIndices;


Have you tested if there's any perf hit to using vector instead of array here?

There is, going from ~200fps to ~80fps in debug mode with three characters on screen, but frankly we should be doing these transformations in the vertex shader anyway...
Lemme know if I should look into that in this PR already or if this can wait until the full-scale refactor

We should leave it as an array for now then and move the transformation into the vertex shader later

Oh wait I think I misunderstood your comment. I think most of the performance hit came from the matrix multiplication; I doubt the switch to vector really did anything noticable. Could profile a little more closely later

Ah okay, you should also probably do profiling in optimized build, matrix multiplication will probably be a lot faster there. There's also probably some SIMD that gets done by default too

Yeah okay, I did some more profiling and it looks to me like the vectorization doesn't really seem to be an impact at all (as in, the loading of the vertices is so fast visual studio can't even process anything meaningful).

I've also checked the matrix multiplications in in release mode, and the fps in the cclcc system menu (which thus far is the worst case scenario I could find) goes from ~800fps on master down to ~340fps, which is a perf hit, but it's still 340fps, and again should be moved to the vertex shader anyway

Yeah we should just move it to the vertex shader. I'd like to see what the numbers are on a worse device like a steam deck though, I assume your computer is probably pretty beefy, so if the perf is a 60% hit on a crappy device it might be a lot worse

KKhanhH · 2025-05-08T04:39:44Z

-        NegativeDelusionSprite, glm::vec2{topLeftX, -109.0f},
-        glm::vec2{424.0f, 557.0f},
-        {1.0f, 1.0f, 1.0f, (float)spinAlpha / 256.0f}, {1, 1}, spinAngle);
+    const glm::vec2 topLeft = {


These probably shouldn't be named topLeft or center and more like offset or something, I can't remember if these names were actually accurate or not or something i just put in.

KKhanhH · 2025-05-10T23:04:06Z

Vulkan seems to be artifacting a lot, it's especially noticable on chlcc, but I'm not sure if it was always like that

PringlesGang · 2025-05-10T23:11:52Z

Vulkan seems to be artifacting a lot, it's especially noticable on chlcc, but I'm not sure if it was always like that

Yeah, Vulkan is very jittery on master too, so I tried not to make things any worse, but also didn't really make an effort to make it much better either. Would need to do a deep dive into learning Vulkan for that at some point

KKhanhH · 2025-06-12T08:40:16Z

-    Renderer->DrawVertices(SystemMenuBG.Sheet, SpriteGridVertices,
-                           DisplayGridVertices, 21, 11);
+    const CornersQuad bgDisp =
+        CornersQuad(BGDispOffsetTopLeft, BGDispOffsetBottomLeft,


brace initialize instead

KKhanhH · 2025-06-12T08:54:51Z

+                               bool isInverted, bool useMaskAlpha) override;
+
+  void DrawVertices(const SpriteSheet& sheet,
+                    std::optional<const SpriteSheet> mask,


This will always be a copy, for a const object it would make more sense to get a reference somehow, pointer or maybe a typedefed optional<reference_wrapper>?

You could also just do default constructible spritesheet and validate it using texture id or something

I'm thinking it might be better to just make it a pointer instead for consistency with everything else (also the draw text functions are already nullable pointers)

KKhanhH · 2025-06-12T09:09:26Z


  // ShaderCompiler compiler
-  ShaderCompiler* Shaders;
+  std::shared_ptr<ShaderCompiler> Shaders;


The lifetime of Shaders isn't unknown or ambiguous, so a shared_ptr isn't necessary here. Scene3D is the only user and it's a member of renderer, so its lifetime will be tied to renderer. Try to avoid shared_ptr unless necessary cuz it does have a perf hit with reference counting + atomic operations.

I'd say just stick to unique_ptr (owner) + ref/raw ptr(users) as a general pattern, but here i think you might be able to just store the value itself and pass references to children

KKhanhH · 2025-06-12T09:14:57Z

-  void QuadSetPosition3DRotated(RectF const& transformedQuad, float depth,
-                                glm::vec2 vanishingPoint, bool stayInScreen,
-                                glm::quat rot, uintptr_t positions, int stride);
+  std::unique_ptr<SpriteShader> SpriteShaderProgram;


These are all different types and can just be stored as values

Use optional if late initialization is needed

KKhanhH · 2025-06-12T09:22:23Z

+  const GLint TexturesLocation;
+};
+
+struct SpriteInvertedUniforms {


Is this struct necessary

Not strictly, but I thought it useful to give it its own struct anyway in case SpriteInverted ever has to divert from Sprite

KKhanhH · 2025-06-12T09:28:42Z

+struct MaskedSpriteNoAlphaUniforms {
+  bool operator==(const MaskedSpriteNoAlphaUniforms& other) const = default;
+
+  glm::mat4 Projection = glm::mat4();


just do glm::mat4 Projection{}

KKhanhH · 2025-06-12T09:41:10Z

+
+  const size_t indexOffset = IndexBuffer.size();
+  IndexBuffer.resize(IndexBuffer.size() + indices.size());
+  std::transform(


you can avoid the resize with back_inserter on IndexBuffer.end()

Won't that continuously push back all the elements, possibly expanding multiple times, instead of resizing just once?

Yeah you can reserve in that case for the allocation, using back_inserter lets you avoid the default construction

KKhanhH · 2025-06-12T09:55:17Z

+  static constexpr size_t TextureUnitCount = 15;
+  static constexpr std::array<GLint, TextureUnitCount> Textures = {
+      0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14};
+  std::array<std::unique_ptr<TextureUnit>, TextureUnitCount> TextureUnits;


Does this need to be unique_ptrs? You might be able to store TextureUnits or optional TextureUnits instead

TextureUnits don't have a default constructor, so I needed late initialization. Optional is possible tho

KKhanhH · 2025-06-12T10:01:32Z

+  void Reserve() { Info->Flushed = false; }
+
+ private:
+  const GLuint Unit;


If it's meant to be or copyable, maybe don't have member variables as const since those kill the assignment operators

KKhanhH · 2025-06-12T10:03:41Z

+    // Add outline to the front of the buffers
+    vertices.insert(vertices.end(), vertices.begin(), vertices.end());
+
+    indices.resize(indexCount * 2);


back_inserter

PringlesGang · 2025-06-12T11:45:24Z

(Fyi I only rebased, so you'll only have to check the five added commits)

…itteeOfZero#165)

Fix code style issues with clang_format Size and position correction Proper .gitignore C;H LCC SystemMenu Cleanup almost perfect CHLCC SystemMenu Added MenuLoopDuration to lua config files Cleanup Fixed error after Renderer transformation refactor (pull request CommitteeOfZero#165) remove comments

PringlesGang self-assigned this Apr 27, 2025

PringlesGang added this to the 0.6.X milestone Apr 27, 2025

KKhanhH reviewed Apr 29, 2025

View reviewed changes

Comment thread src/renderer/renderer.h

KKhanhH reviewed Apr 29, 2025

View reviewed changes

Comment thread src/util.cpp Outdated

Enorovan reviewed Apr 30, 2025

View reviewed changes

Comment thread profiles/common/scriptvars.lua Outdated

PringlesGang force-pushed the renderer-transformation-refactor branch from 494045a to e357f84 Compare May 3, 2025 17:59

PringlesGang marked this pull request as ready for review May 7, 2025 23:00

KKhanhH requested changes May 10, 2025

View reviewed changes

KKhanhH reviewed May 10, 2025

View reviewed changes

Comment thread src/background2d.cpp

PringlesGang force-pushed the renderer-transformation-refactor branch from 7268747 to a10c44a Compare May 12, 2025 22:38

PringlesGang force-pushed the renderer-transformation-refactor branch from 75e60b8 to 89de8c2 Compare June 5, 2025 23:28

newo-2001 mentioned this pull request Jun 11, 2025

CHLCC intro sequence #78

Merged

KKhanhH requested changes Jun 12, 2025

View reviewed changes

PringlesGang force-pushed the renderer-transformation-refactor branch from a47766e to b6bcc1e Compare June 12, 2025 11:44

PringlesGang requested a review from KKhanhH June 12, 2025 11:48

PringlesGang force-pushed the renderer-transformation-refactor branch from b6bcc1e to b6ff901 Compare June 12, 2025 18:58

PringlesGang added 10 commits June 23, 2025 22:09

Moved transformation functions to util

18489da

Added ScrWorkAngleToRad function to util

ad2d530

Moved renderer Quad methods to static base renderer methods

8705e53

Added transformation DrawSprite methods to renderer

250accc

Changed Background2D to use quaternions

cefbc63

Fix rendering issues

535c605

Fixed quad orientation

fef6ee0

Added TODO notes

9cd2ddf

Added Transformation2D function

2baebc4

Added AxisAnglesToQuaternion function

0e04d15

PringlesGang added 21 commits June 23, 2025 22:09

Added masked DrawVertices support

2f7b99b

Added cache check to opengl scissor state

a615aa1

Fix YUV frames

e88cfaf

Turned texture units into managed objects

b235d23

Fix disable blending option

6dbd9c0

Fix basic font rendering

16885a5

Fix chlcc menu background

7575cfb

Fixed MaskedSpriteShader's IsSameTexture uniform not being updated

60b7313

'Fixed' DX9 and VK

0a9662c

Change vertex/index buffers to use vectors instead of arrays

44c14ff

Added override for cclcc system menu

7486955

Fix OpenGL DrawMaskedSpriteOverlay mask UVs

6d18e54

Altered pointer use

d817132

Changed some initializations to brace initializations

b45a557

Removed some resize calls in favor of back_inserter

cb19bc0

Added TextureUnit assignment operators

7da1885

Changed DrawVertices to take in a nullable Sprite* for the mask

f0dfb67

Turn sampler2D[] back into sampler2D uniforms

9ae2016

Fix scrollbar not rendering track when it doesn't have one

6db5d11

Added masked DrawVertices support for Vulkan

a43da76

Added masked DrawVertices support for DirectX9

27e9e62

PringlesGang force-pushed the renderer-transformation-refactor branch from 1d16a0e to 27e9e62 Compare June 23, 2025 20:10

KKhanhH approved these changes Jun 24, 2025

View reviewed changes

PringlesGang merged commit 3781706 into master Jun 24, 2025
11 checks passed

PringlesGang deleted the renderer-transformation-refactor branch June 24, 2025 21:49

Nergon123 added a commit to Nergon123/impacto that referenced this pull request Jun 26, 2025

Fixed error after Renderer transformation refactor (pull request Comm…

789f991

…itteeOfZero#165)

Nergon123 added a commit to Nergon123/impacto that referenced this pull request Jun 28, 2025

Fixed error after Renderer transformation refactor (pull request Comm…

6f866a6

…itteeOfZero#165)

Enorovan mentioned this pull request Jul 2, 2025

Fix backlog name outline #145

Closed

Uh oh!

Conversation

PringlesGang commented Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PringlesGang commented May 7, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KKhanhH commented May 10, 2025

Uh oh!

PringlesGang commented May 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PringlesGang commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

PringlesGang commented Apr 27, 2025 •

edited

Loading

PringlesGang commented Jun 12, 2025 •

edited

Loading